-
Notifications
You must be signed in to change notification settings - Fork 3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[webview_flutter_wkwebview] Updates the internal wrapper to use @ProxyApi
from pigeon
#8311
base: main
Are you sure you want to change the base?
Conversation
@ProxyApi
from pigeon
return instance | ||
} | ||
|
||
#if compiler(>=6.0) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@MainActor
was required for local development with Xcode 16, but our CI doesn't recognize the method because it uses XCode 15. Related to: flutter/flutter#148870
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Huh, I thought Swift 6 mode, with strict concurrency checking, was opt-in for Xcode 16.
/// Creates an error when a method is called on an unsupported version. | ||
func createUnsupportedVersionError(method: String, versionRequirements: String) -> PigeonError { | ||
return PigeonError( | ||
code: "FWFUnsupportedVersionError", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Keeps error code consistent with previous implementation.
// Use of this source code is governed by a BSD-style license that can be | ||
// found in the LICENSE file. | ||
|
||
#if os(iOS) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note: These could be changed to canImport(UIKit)
so that other platforms, like tvOS, could be used without changes. But I went with what was used in the Objective-C
implementation.
/// | ||
/// Since `URLRequest` is a struct, it is pass by value instead of pass by reference. This makes | ||
/// it not possible to modify the properties of a struct with the typical ProxyAPI system. | ||
class URLRequestWrapper { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting FYI for anyone curious and additional context that I can add to the comment if needed:
The Swift URLRequest
represents the Objective-C NSURLRequest
and MutableNSURLRequest
and converts to one or the other depending on the context.
This is different from URL
and NSURL
, which doesn't have a mutable equivalent. So a URL
would always convert to the same NSURL
instance when needed.
This distinction might come up when wrapping other structs.
/// change of the plugin. Native code other than this external API does not follow breaking change | ||
/// conventions, so app or plugin clients should not use any other native APIs. | ||
@objc(WebViewFlutterWKWebViewExternalAPI) | ||
public class FWFWebViewFlutterWKWebViewExternalAPI: NSObject { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This class was converted to Swift, but the @objc
annotations should provide the equivalent method (I wrote the swift test before creating the class).
8FB79B842820A3A400C101D3 /* FWFUIDelegateHostApiTests.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; name = FWFUIDelegateHostApiTests.m; path = ../../darwin/Tests/FWFUIDelegateHostApiTests.m; sourceTree = SOURCE_ROOT; }; | ||
8FB79B8E2820BAB300C101D3 /* FWFScrollViewHostApiTests.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; name = FWFScrollViewHostApiTests.m; path = ../../darwin/Tests/FWFScrollViewHostApiTests.m; sourceTree = SOURCE_ROOT; }; | ||
8FB79B902820BAC700C101D3 /* FWFUIViewHostApiTests.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; name = FWFUIViewHostApiTests.m; path = ../../darwin/Tests/FWFUIViewHostApiTests.m; sourceTree = SOURCE_ROOT; }; | ||
8FB79B962821985200C101D3 /* FWFObjectHostApiTests.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; name = FWFObjectHostApiTests.m; path = ../../darwin/Tests/FWFObjectHostApiTests.m; sourceTree = SOURCE_ROOT; }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@stuartmorgan How were you able to get a relative path for the files in darwin/Tests
? I had to manually change all the paths in this file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I'm doing it in the UI (sometimes I just edit the file manually instead), once they are moved I click on the broken file entry, press the folder icon under Location in the right-hand sidebar, select the file, then change the Location drop-down to "Relative to Project" (the last step is optional, but, I think project-relative makes the most sense).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The Swift 5/6 #if
duplications are pretty unfortunate; hopefully we won't be stuck in this situation for too long.
## Package Structure | ||
|
||
This plugin serves as a platform implementation plugin as outlined in [federated plugins](https://docs.flutter.dev/packages-and-plugins/developing-packages#federated-plugins). | ||
The sections below will provide an overview of how this plugin implements this portion with Android. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"with Android" -> "for iOS and macOS".
### Quick Overview | ||
|
||
This plugin implements the platform interface provided by `webview_flutter_platform_interface` using | ||
the native WebKit APIs for Android. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"WebKit APIs for Android" -> "WKWebView APIs".
|
||
##### 1. Ensure the project has been built at least once | ||
|
||
Run `flutter build ios --simulator` in `example/`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this actually necessary to run pigeon
? I would only expect to need flutter pub get
.
needs to be done. Alternatively, it can be easier to update native code with the platform's specific | ||
IDE: | ||
|
||
Open `example/android/` in a separate Android Studio project. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Open example/ios/Runner.xcworkspace
or example/macos/Runner.xcworkspace
in Xcode.
@@ -27,30 +27,9 @@ | |||
33CC10F32044A3C60003C045 /* Assets.xcassets in Resources */ = {isa = PBXBuildFile; fileRef = 33CC10F22044A3C60003C045 /* Assets.xcassets */; }; | |||
33CC10F62044A3C60003C045 /* MainMenu.xib in Resources */ = {isa = PBXBuildFile; fileRef = 33CC10F42044A3C60003C045 /* MainMenu.xib */; }; | |||
33CC11132044BFA00003C045 /* MainFlutterWindow.swift in Sources */ = {isa = PBXBuildFile; fileRef = 33CC11122044BFA00003C045 /* MainFlutterWindow.swift */; }; | |||
33CF716F2C090A5900FB3AA4 /* FWFURLProtectionSpaceHostApiTests.m in Sources */ = {isa = PBXBuildFile; fileRef = 33CF715A2C090A5800FB3AA4 /* FWFURLProtectionSpaceHostApiTests.m */; }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did the Swift tests get dropped from the macOS project? I only see the removals here.
return webView.addObserver(observer, keyPath, options); | ||
case NSViewWKWebView(): | ||
return webView.addObserver(observer, keyPath, options); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All these switches to make identical calls are unfortunate. We'll need to think about better ways for interop systems to handle this kind of thing longer term.
s.source_files = 'webview_flutter_wkwebview/Sources/webview_flutter_wkwebview/**/*.{h,m}' | ||
s.public_header_files = 'webview_flutter_wkwebview/Sources/webview_flutter_wkwebview/include/**/*.h' | ||
s.module_map = 'webview_flutter_wkwebview/Sources/webview_flutter_wkwebview/include/FlutterWebView.modulemap' | ||
s.source_files = 'webview_flutter_wkwebview/Sources/webview_flutter_wkwebview/**/*.{h,swift}' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we still have .h files?
s.ios.dependency 'Flutter' | ||
s.osx.dependency 'FlutterMacOS' | ||
s.ios.deployment_target = '12.0' | ||
s.osx.deployment_target = '10.14' | ||
s.pod_target_xcconfig = { 'DEFINES_MODULE' => 'YES' } | ||
s.resource_bundles = {'webview_flutter_wkwebview_privacy' => ['webview_flutter_wkwebview/Sources/webview_flutter_wkwebview/Resources/PrivacyInfo.xcprivacy']} | ||
s.pod_target_xcconfig = { 'DEFINES_MODULE' => 'YES', 'EXCLUDED_ARCHS[sdk=iphonesimulator*]' => 'i386' } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we still need EXCLUDE_ARCHS? I thought that was a legacy thing that we didn't need to include any more.
let identifier: Int64 = args is Int64 ? args as! Int64 : Int64(args as! Int32) | ||
let instance: AnyObject? = instanceManager.instance(forIdentifier: identifier) | ||
|
||
if let instance = instance as? FlutterPlatformView { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we not know what type the view identifier is for?
return instance | ||
} | ||
|
||
#if compiler(>=6.0) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Huh, I thought Swift 6 mode, with strict concurrency checking, was opt-in for Xcode 16.
Also fixes flutter/flutter#152352
Pre-launch Checklist
dart format
.)[shared_preferences]
pubspec.yaml
with an appropriate new version according to the pub versioning philosophy, or this PR is exempt from version changes.CHANGELOG.md
to add a description of the change, following repository CHANGELOG style, or this PR is exempt from CHANGELOG changes.///
).If you need help, consider asking for advice on the #hackers-new channel on Discord.